fix: interpretations modal height (DHIS2-15558) #1549
Conversation
There was a problem hiding this comment.
This fixes a big UI issue, but I believe it does create a much smaller issue as well. Currently if we would show a table with a small height in the interpretations modal, the modal would not contain much whitespace. Since we now have a height of calc(100vw - 128px), there will be whitespace whenever the table has limited height.
On balance I'd say that the implementation in this PR is better than what we have: looking at some whitespace is a lot less of a problem than looking at a badly squashed graph. So I am approving.
But I am also wondering if perhaps it would be a good idea to have a prop which would allow us to switch between the auto-height and fixed-height implementation, since for tables auto-height is better, while for graphs the fixed height is required.
| @@ -24,21 +24,22 @@ const modalCSS = css.resolve` | |||
| max-width: calc(100vw - 128px) !important; | |||
| max-height: calc(100vh - 128px) !important; | |||
| width: auto !important; | |||
| height: auto !important; | |||
| height: calc(100vw - 128px) !important; | |||
There was a problem hiding this comment.
| height: calc(100vw - 128px) !important; | |
| height: calc(100vh - 128px) !important; |
|
Superseded by the combo PR #1588 |
Implements DHIS2-15558
Relates to https://github.com/dhis2/data-visualizer-app/pull/XXX
Key features
Description
Previously the interpretations modal didn't have a min height, so the modal could become unusably short. This was especially problematic in DV, as visualisations there adapt to the height of the container (modal). This fix sets the height to the max available height in the viewport.
In addition this fix also addresses a weird scroll behaviour where the top and bottom the interpretations modal would scroll as well. The top and bottom are now fixed and only the list of messages scroll instead.
Known issues
Screenshots
before vs after
interpretations.modal.mov
before: height
after: height
before: scroll
after: scroll